-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a "cancel" event for when file upload selection is unchanged #6735
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This fulfills the use case I had in mind in #6376!
With regards to event bubbling, I prefer API surfaces that are consistent rather than following the practices at the time each new feature is added, and I think that would be less confusing for web developers (though I'm not sure who would attach event handlers to a parent element rather than the input
element itself). I'm a bit uneducated as to how event bubbling works, but are there any cancel
events that could already be emitted by parent elements and listened for, that could cause compatibility issues if the input
element starts emitting them as well and bubbling?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
I would be okay with this being a composed event as I mentioned in that issue for its counterpart, but it's probably best to decide on them jointly.
cc @smaug----
@mfreed7 do you want to review this? Any thoughts especially on the "Specific choices made" section would be welcome. |
|
||
<li><p>Wait for the user to have made their selection.</p></li> | ||
|
||
<li><p><span>Update the file selection</span> for the <code>input</code> element.</p></li> | ||
<li><p>If the user dismissed the prompt without changing their selection, then <span>queue an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to be more specific here? As written, this would fire "cancel" if a user selected a file, clicked OK, re-opened the file picker, selected the same file again, and clicked OK. In that case, the selection "would not be changed". Is that intentional? If so, perhaps this case could be explicitly folded into the new note below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that's right. Such a case would fire "cancel" and not "change", since nothing changed. I'll add it to the example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, well, it kind of depends; something might have changed, since maybe the files changed on disk? But I don't think the event should need to branch on that.
So I guess now I'm thinking it's up to the UA. As long as it picks one of the two events it's OK. I'll try to come up with some phrasing...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thinking about this a bit more, I'm thinking the more correct thing would be to fire change
in this case (re-selecting the same file). The issue is that otherwise, a developer would have no way to detect this situation. There are two possibilities:
- Re-selection triggers
cancel
: There is no way to disambiguate re-selection from hitting cancel. - Re-selection triggers
change
: The developer can, if they like, store the previously-selected file, and compare them upon receipt ofchange
.
It seems plausible that a developer might want to do something different when the user clicks "ok" vs "cancel".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think that's up to the UA. It seems like a valid user interface to say that if the user didn't change anything (i.e. re-selected the same files), then that shouldn't be distinguishable from hitting cancel.
Overall this looks pretty good to me. I added a question about re-selecting the same file. And here are my thoughts on the choices:
I think "cancel" makes sense, both because of parallelism with other APIs but also because it maps to the name of the button ("Cancel") used by most UAs.
I was surprised by this one, I would have thought we'd set
From our last spec triage meeting, I think the consensus was that |
I'll change it to bubble since we've gotten a couple of small pieces of feedback in that direction. |
Follow-up question: is there a way to feature-detect support for this? My initial approach… 'oncancel' in (i = document.createElement('input'), i.type = 'file', i)
// `true` …was not helpful. |
I don't believe so, no. |
The event on `<input type=file>` was missing and implemented only in Firefox: whatwg/html#6735 https://bugs.chromium.org/p/chromium/issues/detail?id=1227424 https://bugzilla.mozilla.org/show_bug.cgi?id=1719703 https://bugs.webkit.org/show_bug.cgi?id=227799
The event on `<input type=file>` was missing and implemented only in Firefox: whatwg/html#6735 https://bugs.chromium.org/p/chromium/issues/detail?id=1227424 https://bugzilla.mozilla.org/show_bug.cgi?id=1719703 https://bugs.webkit.org/show_bug.cgi?id=227799 Co-authored-by: Queen Vinyl Da.i'gyu-Kazotetsu <[email protected]>
I'm curious if it was intended that this change also applied to a drag/drop action that leaves the file selection unchanged. The current spec text for that says:
which seems to imply that cancel events should never fire for that case. This difference seems a little odd... and also seems to expose the mechanism the user used to change the selection, which may be undesirable. |
Interesting question. When would you expect the cancel event to fire? After all, most of the time the user is not dragging-and-dropping files. It seems like it's the two-stage nature of (1) opening a picker, (2) then closing it, which lets us definitively decide at (1) that we're definitely going to fire an event, and at (2) which event to fire. Dropping is a single action, so I don't know how we would determine this... |
I guess if the user drops the same set of files as the current set, I'd expect either (a) a cancel event or (b) no events at all to fire at that time. Firing an input and a change event when the files didn't change exposes (I think) that the action was drag/drop rather than via the picker. (Also, for the file picker case, Chromium, at least, fires the input and the change events at the same time, after the user has closed the picker.) (Also, for the picker case, I don't recall if there are currently APIs that allow the page to open the file picker, but if there are, that's a good reason that a |
(Or, to more directly answer the question, I'd expect that |
Not sure if it's immediately relevant, but I just encountered an interesting corner case: pointing an |
I should also note that Chromium uses the same code to handle both receiving dropped files and a result from the file chooser. So fixing the latter to fire (Not sure about the |
Also, the older version of https://chromium-review.googlesource.com/c/chromium/src/+/4048886 has a test that might be worth adding to WPT if this discussion concludes. |
Filed #8945. |
See html spec [1] [2]. If the file upload selection for input type=file is unchanged, we should fire the cancel event with bubbles true. [1] https://html.spec.whatwg.org/#show-the-picker,-if-applicable [2] whatwg/html#6735 Change-Id: I8fb0307df75c58c17ff1d180071cd77634a08a65 Fixed: 1227424 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4048886 Commit-Queue: Di Zhang <[email protected]> Reviewed-by: David Baron <[email protected]> Cr-Commit-Position: refs/heads/main@{#1109778}
How should I detect whether the browser supports this event? |
Closes #6376.
Specific choices made:
<dialog>
) and it seems nicer to reuse that than introduce something confusingly-similar (like "abort").I leftFlipped based on below feedback.bubbles
as false since most modern events don't seem to bubble. Maybe this is confusing though since both change and input bubble. I'm very open to changing this.composed
as false. Interestinglyinput
is composed butchange
is not.input
event is composed butchange
isn't #5453 I'm not sure what the right thing to do here is.(See WHATWG Working Mode: Changes for more details.)
/indices.html ( diff )
/input.html ( diff )